fix: make Noema review independent - #1477

… cap no-op bug

Rebases this branch onto main's provider_account/account_cap rename (#1468)
and Noema-independence work (#1477/#1480), then fixes a real bug this
branch's own batched-preflight merge introduced: _catalog_family_cap()
defaulted to REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES (24) whenever
ORCHESTRATOR_CATALOG_FAMILY_CAP was unset, silently disabling per-account
diversification. Live production evidence: probed_count 12, ready_count 2,
rejected_count 10 (83% rejected via 429/404/timeout), with the admitted
free-pool catalog 100% nvidia_nim/nvidia_nim_sub -- two credentials sharing
one rate-limited upstream jointly occupying the entire 12-slot preflight
batch. Reported at
#1415 (comment)

Fix (mirrors the pattern in open main PR #1487's diff, not yet merged):
renamed the helper to _catalog_account_cap(default), which now requires the
caller to supply contextual_orchestrator_review_policy.DEFAULT_ACCOUNT_CAP
(4) as the default instead of hand-typing/defaulting to a total-routes-scale
constant. An explicit ORCHESTRATOR_CATALOG_ACCOUNT_CAP override remains
honored. The sidecar shell script's equivalent CATALOG_FAMILY_CAP default
(also baked to the total 24-route budget) is renamed to CATALOG_ACCOUNT_CAP
and restored to a real, smaller cap (8, main's existing operational value).

Also corrects CHANGELOG.md and docs/adr/0003 prose that had described the
no-op 24-route cap as an intentional fix, and updates two contract tests
(test_contextual_orchestrator_review_runtime_preflight.py,
test_contextual_orchestrator_review_sidecar_contract.py) that pinned the old
family_cap naming and the buggy default.

Verification: coverage run -m pytest tests -> 2122 passed, 1 skipped, 21
subtests; coverage report -> 100% on scripts/ci; interrogate -> 100%
docstrings; bash -n on the touched shell script; git diff --check clean.
Red-then-green: reverting _catalog_account_cap's default back to
REVIEW_PREFLIGHT_MAX_TOTAL_ROUTES makes the new regression tests fail;
restoring the fix makes them pass again.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX

Devin Review (this PR, scripts/ci/contextual_orchestrator_review_launcher.py
lines 129-169) flagged that the docstrings describe
#1477 (the ORCHESTRATOR_PIN_SHA bump to
contextual-orchestrator#949's merge commit) as still pending. Verified: #1477
merged on 2026-08-31 -- before this PR's original evidence_only/spend_admitted
fix even landed -- and `git merge-base --is-ancestor
8cd99f139915131ba0239bce12a5d6a5fd85394e 045d17da5e2aea56a97e241ee158ab1628d78660`
against contextual-orchestrator confirms the current pin already descends from
#949's fix commit. The "not yet pinned" / "once #1477 merges" framing was
therefore stale runtime history that would mislead future maintenance into
thinking the pin bump was still outstanding.

Updates both affected docstrings in the launcher module and the two
docstrings in tests/test_contextual_orchestrator_review_runtime_preflight.py
that repeated the same stale framing. Docs-only: no behavior change, no
assertion changed. docs/product-technical-gap-baseline.md's own #1477
references are left untouched -- they are dated, explicitly time-qualified
("open as of this correction") historical entries in this repo's append-only
decision log, not living claims about current state, so rewriting them would
falsify the audit trail rather than correct it.

Verified: `python -m pytest tests/test_contextual_orchestrator_review_runtime_preflight.py
tests/test_contextual_orchestrator_review_sidecar_contract.py -q` -- 95 passed;
`interrogate scripts/ci/contextual_orchestrator_review_launcher.py` -- 100%.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw

Two conflicted files, four blocks. The branch's delta splits into two
independent parts with opposite verdicts.

PART A - paginated review evidence: KEPT (49 of the branch's 52 added
script lines).
  main still fetches reviews through GraphQL 'reviews(last: 100)'
  (noema_review_gate.py:324), so a review event older than the last 100
  is invisible to existing_noema_review() - main's only consumer of
  pr['reviews']['nodes'] - and Noema re-reviews a PR it already reviewed.
  fetch_complete_reviews() and its fetch_pr wiring merged cleanly outside
  every conflict block and are unchanged here.

PART B - fail-closed guard before Noema skip paths: DROPPED as superseded
(3 of 52 lines).
  a643d8a 'fix: make Noema review independent (#1477)' deleted all four
  helpers this guard is built on. Verified org-wide, not just in this
  file: current_primary_approval, has_current_changes_requested,
  has_unresolved_threads and blocking_checks each return 0 hits across
  all of origin/main (positive control: require_expected_head, 16 hits).
  main closed the same hole by removing the skip paths rather than
  guarding them, and made the primary-actor case raise RuntimeError where
  the branch only returned 1 - strictly stronger than this PR's fix.
  The branch's own test also called the pre-#1477 signature
  (inspect_and_review missing required 'expected_head'), so it was
  superseded twice over.

Block verdicts:
- noema_review_gate.py x2  -> main's inspect_and_review structure. The
  branch's guard has no surviving call target.
- test file, block 1       -> main's test_existing_noema_review_matches_
  actor_and_head, plus the branch's two Part A tests appended at EOF (the
  conflict boundary cut through the middle of main's function, so they
  cannot be inserted at the block site).
- test file, block 2       -> main's parametrized cases; the branch's used
  make_pr shapes for deleted behaviour.
- test_noema_never_skips_success_without_current_head_primary_approval
  merged cleanly outside the blocks and was removed: it is Part B.

One assertion was translated, not dropped:
test_fetch_pr_keeps_exact_head_approval_older_than_one_hundred_reviews
asserted through the deleted current_primary_approval(). It now asserts
main's live path directly - len(pr['reviews']['nodes']) == 101 and
nodes[0] is the pre-page-boundary approval - which is the test's stated
purpose and keeps its detection power.

Evidence:
- uvx ruff check --select F821 scripts/ci tests: All checks passed
- full suite, branch head b8f4adb (unmerged): 2099 passed, 0 failed
- full suite, this merge:                      2899 passed, 0 failed
  (failure-name diff empty both ways, denominators 2099 and 2899)
- negative control: deleting the fetch_pr REST wiring makes the kept
  pagination test fail by name -> it detects the regression it claims to
- coverage: TOTAL 100%, noema_review_gate.py 860/860 100%
- interrogate: PASSED (minimum 100.0%)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…-20260824)

13 conflict blocks across four files. Both sides hardened the same function
independently - this branch at the credential egress boundary, main at output
validation and gateway telemetry (#1477) - so the resolution is a union, not a
choice, except where the two guards overlap.

THE OVERLAP, resolved in the branch's favour with evidence:
  main's reject_private_llm_url has a fail-OPEN path - 'except socket.gaierror:
  return' - and does not require HTTPS for non-loopback targets, does not reject
  userinfo, and denylists special addresses instead of requiring globally
  routable unicast. The branch's validate_endpoint is stricter on every one of
  those axes and fails closed on DNS failure, which is this PR's stated subject.
  So call_llm now calls validate_endpoint only; reject_private_llm_url stays
  defined because two tests exercise it directly and still pass.
  That makes main's four call_llm-level 'URL cannot target internal IP
  addresses' assertions wrong and the branch's replacements right - which is
  exactly what the branch's side of those four conflict blocks already said.

PORTED onto main's structure:
- opener gains ProxyHandler({}) + PinnedHTTPHandler/PinnedHTTPSHandler in front
  of main's NoRedirectHandler
- response.read(MAX_LLM_RESPONSE_BYTES + 1) with the over-limit check
- post-request DNS identity re-check before decoding
- constants and the pinned-connection classes merged alongside main's JSON
  bound helpers (disjoint symbol sets, so a plain union)

NOT PORTED, on policy:
  The branch also passes timeout=120 to opener.open and its oracle asserts
  'timeout == 120'. CLAUDE.md states model-path timeouts are policy-fixed, and
  #1889/#1890/#1892 each added such a cap and were all reverted (#1891, #1895);
  main's call_llm docstring says it 'carries no fixed model wall-clock
  deadline'. The assertion was translated to 'timeout is None' with that
  citation in a comment. The byte cap is the bound that test really needs.

TEST FIXTURES updated because the behaviour changed, not to make red go green:
- the branch's oracle called the pre-#1477 five-argument call_llm (11 sites) and
  did not stub validate_substantive_verdict, which main added; both translated,
  the latter the same way main's own call_llm tests do it
- fail-closed DNS means an unresolvable *.example.test aborts a test before its
  own assertion, so tests/test_noema_review_gate.py and
  tests/test_noema_repair_attempt_telemetry.py get an autouse fixture that
  resolves non-literal hosts and leaves literal addresses to the real resolver,
  so the internal-address tests keep their meaning
- the byte-capped read passes an argument, so nine response doubles became
  read(self, _limit=None)

Evidence:
- uvx ruff check --select F821 scripts/ci tests: All checks passed
- full suite, branch head 3feb583 (unmerged): 1931 passed, 0 failed
- full suite, this merge:                      2938 passed, 0 failed
- the branch's own oracle tests/test_noema_endpoint_boundary.py: 33 passed
  (10 before the signature translation)
- main's noema tests: 121 passed; telemetry contracts: 23 passed
- negative controls: removing the post-request DNS re-check fails
  test_public_endpoint_requires_https_and_stable_global_dns; removing the byte
  cap fails test_response_body_is_bounded_before_json_decoding - both by name
- coverage: TOTAL 100%; interrogate: PASSED (minimum 100.0%)

GAP FOUND, left for the author: removing PinnedHTTPHandler/PinnedHTTPSHandler
from the opener changes no test result (33 still pass). The pinned classes are
covered only by direct unit tests; nothing asserts they are installed in
call_llm's opener, so a future edit could unwire DNS pinning silently. This is
pre-existing in the PR, not introduced here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>